Skip to content

Fix flaky unknown block test - #10001

Merged
mergify[bot] merged 4 commits into
sigp:unstablefrom
michaelsproul:fix-flaky-unknown-block-test
Sep 9, 2026
Merged

Fix flaky unknown block test#10001
mergify[bot] merged 4 commits into
sigp:unstablefrom
michaelsproul:fix-flaky-unknown-block-test

Conversation

@michaelsproul

Copy link
Copy Markdown
Member

Issue Addressed

Fix flaky CI test:

Proposed Changes

The test was failing due to a race between importing the block on gossip vs importing the block as a result of its getBlobs call completing. The getBlobs codepath was missing a notification to the reprocess queue, which this PR adds.

There's also a new test added which prevents regression on the getBlobs codepath.

Additional Info

Heavily Codex driven, manually reviewed. Change makes sense to me.

If the EL blob fetch wins the block import race, queued attestations never receive the import notification and attestation_to_unknown_block_processed_after_gossip_block can time out. Reuse the column import notification helper so this path also releases waiting work, including payload attestations after Gloas.
Process the Fulu block without its columns before explicitly fetching blobs from the EL. Assert that the block becomes head and its queued attestation is reprocessed and imported. This deterministically fails when the EL import notification is missing.
@michaelsproul
michaelsproul requested a review from jxs as a code owner September 8, 2026 05:30
@michaelsproul michaelsproul added test improvement Improve tests ready-for-review The code is ready for review infra-ci labels Sep 8, 2026
@dknopik
dknopik self-requested a review September 8, 2026 07:51
"Block components retrieved from EL"
);
self.chain.recompute_head_at_current_slot().await;
self.notify_import_after_column(slot, block_root);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also runs for RPC lookups, but the notification helpers log failures with source="gossip". Should we pass the source through so a failed notification after getBlobs isn't reported as gossip?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, will update!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in

43e756c

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 8, 2026
@michaelsproul michaelsproul added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 9, 2026

@jimmygchen jimmygchen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mergify

mergify Bot commented Sep 9, 2026

Copy link
Copy Markdown

Queued — the merge queue status continues in this comment ↓.

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 9, 2026
@mergify

mergify Bot commented Sep 9, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 32 minutes 5 seconds in the queue, including 28 minutes 29 seconds running CI.

Required conditions to merge

@mergify mergify Bot added the queued label Sep 9, 2026
@mergify
mergify Bot merged commit d412188 into sigp:unstable Sep 9, 2026
38 checks passed
@mergify mergify Bot removed the queued label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra-ci low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge. test improvement Improve tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants